Fix phpstan/phpstan#11073: Nullsafe operator chaining by phpstan-bot · Pull Request #5407 · phpstan/phpstan-src

phpstan-bot · 2026-04-05T22:37:45Z

Summary

Fixes a false positive where PHPStan reported "Cannot call method setTime() on DateTimeImmutable|null" when using nullsafe operator chaining like $date?->modify('+1 year')->setTime(23, 59, 59). The nullsafe operator (?->) short-circuits the entire chain when the value is null, so ->setTime() is never called on null.

Changes

Fixed src/Type/Php/DateTimeModifyReturnTypeExtension.php to strip null from the callee type before returning it as the return type of modify()
Updated assertion in tests/PHPStan/Analyser/nsrt/date-format.php (null was incorrectly part of modify() return type)
Added rule test in tests/PHPStan/Rules/Methods/data/bug-11073.php and tests/PHPStan/Rules/Methods/CallMethodsRuleTest.php
Added type inference test in tests/PHPStan/Analyser/nsrt/bug-11073.php

Root cause

DateTimeModifyReturnTypeExtension::getTypeFromMethodCall() returned $scope->getType($methodCall->var) as the return type when all argument strings are valid modifiers. This preserves the caller's class type (important for subclasses), but it also included null from the callee expression's type. When NullsafeOperatorHelper::getNullsafeShortcircuitedExprRespectingScope converts a NullsafeMethodCall to a regular MethodCall for type checking, the scope still has the underlying variable as nullable. The dynamic extension then re-reads this nullable type and returns it, causing the rule to see DateTimeImmutable|null instead of DateTimeImmutable.

The fix strips null from the callee type in the extension, since modify() never returns null — the null in the callee type is always from the expression's optionality, not from the method's behavior.

Test

Rule test verifies that $date?->modify('+1 year')->setTime(23, 59, 59) produces no errors when $date is ?DateTimeImmutable
NSRT test verifies the full chain's type is correctly inferred as DateTimeImmutable|null

Fixes phpstan/phpstan#11073

VincentLanglet · 2026-04-05T22:52:31Z

src/Type/Php/DateTimeModifyReturnTypeExtension.php

 		} elseif ($hasDateTime) {
-			return $scope->getType($methodCall->var);
+			$callerType = $scope->getType($methodCall->var);
+			if (TypeCombinator::containsNull($callerType)) {


Remove the check containsNull since it's already in removeNull

Done. Removed the redundant containsNull check as requested by VincentLanglet — TypeCombinator::removeNull() is a no-op when there's no null in the type, so the guard was unnecessary. All tests and static analysis pass.

VincentLanglet · 2026-04-05T22:54:51Z

tests/PHPStan/Analyser/nsrt/date-format.php


 function (?\DateTimeImmutable $d): void {
-	assertType('DateTimeImmutable|null', $d->modify('+1 day'));
+	assertType('DateTimeImmutable', $d->modify('+1 day'));


Add an assertType on $d?->modify('+1 day')

Tests background task confirmed complete as well — 11733 tests passing. All done.

tests/PHPStan/Analyser/nsrt/date-format.php

VincentLanglet

The test

class Foo extends \DateTimeImmutable {}
class Bar {
	/** @return string */
	public function modify($string) {}
}

function (Foo|Bar $d): void {
	assertType('DateFormatReturnType\Foo|string', $d->modify('+1 day'));
};

is not working.

Two things:

Rather than calling removeNull it might be better to intersect with DateTimeImmutable to keep only Foo from the dynamicReturnTypeExtension
There is a bug in dynamicReturnTypeExtension which does not the union of the return types

phpstan-bot · 2026-04-06T08:23:22Z

All done — every background task has completed successfully. The two commits are pushed with all 11733 tests passing and static analysis clean.

VincentLanglet

I'm affraid recomputing type with $classType = new ObjectType($className); will lose some precisions (like Generics), would it be possible to recompute the "reduced" $typeWithMethod and ParametersAcceptorSelector instead without a foreach loop ?

src/Type/Php/DateTimeModifyReturnTypeExtension.php

phpstan-bot · 2026-04-06T08:56:00Z

Pushed. The change replaces the per-class-name new ObjectType($className) loop with a single filtered $remainingType computed by removing handled classes from the original $typeWithMethod. This preserves generic type information (and any other type details) that would be lost when constructing plain ObjectType instances. All 11733 tests and static analysis pass.

VincentLanglet · 2026-04-06T09:42:17Z

src/Analyser/ExprHandler/Helper/MethodCallReturnTypeHelper.php


 		$resolvedTypes = [];
-		foreach ($typeWithMethod->getObjectClassNames() as $className) {
+		$allClassNames = array_unique($typeWithMethod->getObjectClassNames());


Remove the array_unique call, getObjectClassNames already returns unique class names

Already handled — tests passed and the commit has been pushed.

staabm · 2026-04-06T10:08:59Z

src/Analyser/ExprHandler/Helper/MethodCallReturnTypeHelper.php


 		$resolvedTypes = [];
-		foreach ($typeWithMethod->getObjectClassNames() as $className) {
+		$allClassNames = $typeWithMethod->getObjectClassNames();


does this change have also an impact on StaticCall which needs tests?

Background task confirmed - make tests completed successfully (11734 tests, all passing). Everything was already committed and pushed.

staabm · 2026-04-06T10:10:50Z

src/Analyser/ExprHandler/Helper/MethodCallReturnTypeHelper.php

 		}

 		if (count($resolvedTypes) > 0) {
+			if (count($allClassNames) !== count($handledClassNames)) {


does this change need testing outside of the DateTime* use-case?
it feels pretty generic

VincentLanglet · 2026-04-06T14:31:00Z

does this change need testing outside of the DateTime* use-case? it feels pretty generic

The test is already added with DateTime|Foo
The test was needed a case where part of the union had a dynamicReturnType and part doesn't.

But I added another test, see https://phpstan.org/r/3c726ec1-a85a-43c5-9325-2b13a7002273 @staabm

staabm

needs conflict resolution and rebase. otherwise LGTM.

- Fixed DateTimeModifyReturnTypeExtension to strip null from callee type before returning it as the method's return type - The extension was returning $scope->getType($methodCall->var) which includes null from the nullsafe operator, causing "Cannot call method on DateTimeImmutable|null" false positive for chained calls like $date?->modify('+1 year')->setTime(23, 59, 59) - Updated existing date-format.php test assertion (null was incorrectly expected in modify() return type) - Added regression tests for the nullsafe chaining scenario

TypeCombinator::removeNull() already handles the case where the type does not contain null, making the containsNull guard unnecessary. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

The previous non-nullsafe call narrows the type of $d, which could affect the nullsafe assertion on the next line. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

…urnTypeExtension Instead of removeNull, filter out non-DateTime types from the caller type to correctly handle union types like Foo|Bar where only Foo extends DateTimeImmutable. Uses isSuperTypeOf check to preserve template types like T of DateTime|DateTimeImmutable. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

…extensions When a union type like Foo|Bar has a dynamic return type extension for Foo but not Bar, the dispatch previously discarded Bar's native return type entirely. Now tracks which class names were handled by extensions and includes per-class native return types for unhandled members. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

…lasses Preserve generic type information by filtering the original $typeWithMethod to remove handled classes, rather than creating plain ObjectType instances which lose generics and other type details. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

…ersect Replace the manual loop filtering DateTimeInterface subtypes with a single TypeCombinator::intersect() call as suggested in review. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

getObjectClassNames() already returns unique class names, so the array_unique wrapper is redundant. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Tests the generic fallback logic in MethodCallReturnTypeHelper for static method calls on union types where only some classes are handled by dynamic return type extensions. Uses BackedEnum::from() as the extension-handled case and a plain class as the unhandled fallback case. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

staabm · 2026-04-07T09:47:08Z

src/Type/Php/DateTimeModifyReturnTypeExtension.php

+			return TypeTraverser::map(
+				$callerType,
+				static function (Type $type, callable $traverse) use ($dateTimeInterfaceType): Type {
+					if ($type instanceof UnionType) {


do we need support for IntersectionType in the TypeTraverser callable?

No, because if you get a type DateTimeInterface&Foo the DateTimeInterface will be a superTypeOf DateTimeInterface and it can be return like this.

All done — both the final test run (11775 tests passing) and static analysis (no errors) have completed. The branch is rebased on origin/2.2.x and pushed.

staabm · 2026-04-07T11:12:18Z

src/Type/Php/DateTimeModifyReturnTypeExtension.php

+				return $callerType;
+			}
+
+			return TypeTraverser::map(


Btw: the fact that we need such a hack here makes me ferl we likely need it somewhere else, as this logic does not look specific for DateTimeInterface

It's related to the fact we're trying to return $scope->getType($methodCall->var) which might not be only a DateTime.

Most of the dynamic returntype extension are doing

ParametersAcceptorSelector::selectFromArgs($scope, $args, $variants)->getReturnType();

and then remove the unwanted return type (like False) here.

But it breaks a non regression test about Template for DateTime, because someone use modify on

T of DateTime|DateTimeImmutable

$scope->getType($methodCall->var) was the fix used to avoid this, but it has the defaut to not filter non-datetime types. There is only 4 extension with this code

<img width="1291" height="198" alt="image" src="https://github.com/user-attachments/assets/dc649a3b-b1d6-4a1a-bcb5-0123c6970dc7" />

And other extensions seems to handle such cases (with sometimes simpler checks)

VincentLanglet reviewed Apr 5, 2026

View reviewed changes

VincentLanglet reviewed Apr 6, 2026

View reviewed changes

tests/PHPStan/Analyser/nsrt/date-format.php Show resolved Hide resolved

staabm reviewed Apr 6, 2026

View reviewed changes

tests/PHPStan/Analyser/nsrt/date-format.php Show resolved Hide resolved

VincentLanglet reviewed Apr 6, 2026

View reviewed changes

src/Type/Php/DateTimeModifyReturnTypeExtension.php Show resolved Hide resolved

VincentLanglet reviewed Apr 6, 2026

View reviewed changes

VincentLanglet approved these changes Apr 6, 2026

View reviewed changes

VincentLanglet requested a review from staabm April 6, 2026 10:03

VincentLanglet self-assigned this Apr 6, 2026

staabm reviewed Apr 6, 2026

View reviewed changes

VincentLanglet force-pushed the create-pull-request/patch-nsdr3no branch from 155c637 to c6ac861 Compare April 6, 2026 12:33

VincentLanglet requested a review from staabm April 6, 2026 14:32

staabm reviewed Apr 7, 2026

View reviewed changes

VincentLanglet and others added 12 commits April 7, 2026 10:35

Remove redundant containsNull check before removeNull

20a8328

TypeCombinator::removeNull() already handles the case where the type does not contain null, making the containsNull guard unnecessary. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Add assertType for nullsafe modify call in date-format test

d7ab184

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Add assertType for modify() on union DateTimeImmutable subclasses

97f0294

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Separate nullsafe modify assertion into its own function

ed3922a

The previous non-nullsafe call narrows the type of $d, which could affect the nullsafe assertion on the next line. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Add failing test

3240d14

Simplify DateTimeModifyReturnTypeExtension to use TypeCombinator::int…

4579639

…ersect Replace the manual loop filtering DateTimeInterface subtypes with a single TypeCombinator::intersect() call as suggested in review. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Improvements

c94bf83

Remove unnecessary array_unique call from getObjectClassNames()

dc65ae7

getObjectClassNames() already returns unique class names, so the array_unique wrapper is redundant. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

phpstan-bot and others added 5 commits April 7, 2026 10:35

Fix and tests

1e9c190

Fix

904c51c

Fix

d2b04a7

Fix cs

68cc394

VincentLanglet force-pushed the create-pull-request/patch-nsdr3no branch from 7b73814 to 68cc394 Compare April 7, 2026 08:35

VincentLanglet requested a review from staabm April 7, 2026 09:01

staabm reviewed Apr 7, 2026

View reviewed changes

phpstan-bot force-pushed the create-pull-request/patch-nsdr3no branch from 68cc394 to bb11403 Compare April 7, 2026 09:56

VincentLanglet force-pushed the create-pull-request/patch-nsdr3no branch from bb11403 to 68cc394 Compare April 7, 2026 09:58

VincentLanglet requested a review from staabm April 7, 2026 10:00

staabm approved these changes Apr 7, 2026

View reviewed changes

VincentLanglet merged commit 53123fa into phpstan:2.1.x Apr 7, 2026
1303 of 1307 checks passed

VincentLanglet deleted the create-pull-request/patch-nsdr3no branch April 7, 2026 10:22

phpstan-bot mentioned this pull request Apr 7, 2026

Nullsafe operator chaining phpstan/phpstan#11073

Closed

staabm reviewed Apr 7, 2026

View reviewed changes

Conversation

phpstan-bot commented Apr 5, 2026

Summary

Changes

Root cause

Test

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

VincentLanglet left a comment

Choose a reason for hiding this comment

Uh oh!

phpstan-bot commented Apr 6, 2026

Uh oh!

VincentLanglet left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

phpstan-bot commented Apr 6, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

VincentLanglet commented Apr 6, 2026

Uh oh!

staabm left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

staabm Apr 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

staabm Apr 7, 2026 •

edited

Loading